Skip to content

chore(solver): eliminate tuning magic-number drift hazards#163

Merged
daharoni merged 1 commit into
mainfrom
chore/solver-tuning-hygiene
Jul 8, 2026
Merged

chore(solver): eliminate tuning magic-number drift hazards#163
daharoni merged 1 commit into
mainfrom
chore/solver-tuning-hygiene

Conversation

@daharoni

@daharoni daharoni commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Numerical-hygiene pass on the solver crate for the pre-publication review — closes the last correctness-adjacent items from the review checklist (fast-grid dedup + the two biexp TODOs) and removes remaining bare-literal / duplicated tuning constants per the no-magic-numbers rule. No behavior change: every default reproduces the previously hardcoded value.

Biexp fast-grid dedup (biexp_fit.rs)

  • Hoist the fast-component grid bounds (0.25/2.0/0.5/8.0 × dt, 0.15 × tau_d) into shared named consts (TRF_*/TDF_*), used by both cold_grid_search and golden_section_refine so the two stages can no longer drift apart.
  • Fixes a real drift: refinement floored tau_r_fast at 0.1 × dt while the grid floored at 0.25 × dt, letting refinement explore a region the grid never searched. Refinement now clamps to the same shared bounds (and the tau_d_fast refine floor is clamped to the grid floor too).
  • Resolve the two long-standing TODOs (sequential ceiling necessity; dt-relative vs fixed-ms bounds) as documented design decisions with rationale, rather than open markers.

Seed / baseline / EMA config-ification

  • Introduce SeedConfig (mad_multiplier, onset_fraction, max_walkback_s, min_peak_distance_s) with Default; find_seed_spikes reads from it instead of bare literals (4.0, 0.10, 1.0, 5.0). FFI binding signatures (seed_trace, seed_kernel_estimate) are unchanged.
  • Home the rolling-baseline percentile as baseline::DEFAULT_BASELINE_QUANTILE, used by both Solver::subtract_baseline and indeca::solve_trace — previously the literal 0.2 was duplicated across two modules (a drift hazard like the fast-grid one).
  • Name the display-only baseline EMA weight as BASELINE_EMA_WEIGHT.

Testing

  • 131 Rust tests pass; 207 Python tests pass (2 skipped).
  • clippy clean under both --features jsbindings and --features pybindings.
  • cargo fmt --all --check clean.

🤖 Generated with Claude Code

Numerical-hygiene pass on the solver crate for the pre-publication review.
No behavior change: every default reproduces the previously hardcoded value.

Biexp fast-grid dedup (biexp_fit.rs):
- Hoist the fast-component grid bounds (0.25/2.0/0.5/8.0 x dt, 0.15 x tau_d)
  into shared named consts (TRF_*/TDF_*), used by both cold_grid_search and
  golden_section_refine so the two stages can no longer drift.
- Fix a real drift: refinement floored tau_r_fast at 0.1 x dt while the grid
  floored at 0.25 x dt, letting refinement search a region the grid never saw.
  Refinement now clamps to the same shared bounds.
- Resolve the two long-standing TODOs (sequential ceiling; dt-relative vs
  fixed-ms bounds) as documented design decisions with rationale.

Seed/baseline/EMA config- ification:
- Introduce SeedConfig (mad_multiplier, onset_fraction, max_walkback_s,
  min_peak_distance_s) with Default; find_seed_spikes reads from it instead of
  bare literals. FFI binding signatures (seed_trace, seed_kernel_estimate)
  unchanged.
- Home the rolling-baseline percentile as baseline::DEFAULT_BASELINE_QUANTILE,
  used by both Solver::subtract_baseline and indeca::solve_trace (was the
  literal 0.2 duplicated across two modules).
- Name the display-baseline EMA weight as BASELINE_EMA_WEIGHT.

Tests: 131 Rust + 207 Python pass; clippy clean under jsbindings and pybindings.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@daharoni daharoni merged commit 50ca2e9 into main Jul 8, 2026
6 checks passed
@daharoni daharoni deleted the chore/solver-tuning-hygiene branch July 8, 2026 21:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant